Skip to content

swig, generator: unify accessor annotations and add custom-access groundwork#3315

Open
martin-belanger wants to merge 2 commits intolinux-nvme:masterfrom
martin-belanger:libnvme-python-bindings-redesign-plan-1
Open

swig, generator: unify accessor annotations and add custom-access groundwork#3315
martin-belanger wants to merge 2 commits intolinux-nvme:masterfrom
martin-belanger:libnvme-python-bindings-redesign-plan-1

Conversation

@martin-belanger
Copy link
Copy Markdown

Summary

This PR contains two small, incremental patches that prepare the codebase for upcoming work on Python/SWIG binding generation.

The goal is to make accessor semantics explicit and machine-readable so downstream tooling can reason about read/write behavior and custom implementations.

Patch 1: Rename annotation

  • Rename !accessors:*!access:*
  • Update generator, headers, and documentation
  • Purely mechanical change (no behavior change)

Patch 2: Explicit access model + custom support

  • Introduce a unified annotation model:

    • !access:read=<mode>,write=<mode>
    • !generate-accessors:read=<mode>,write=<mode>
  • Modes: generated, custom, none

  • Struct-level annotations define defaults; member-level overrides per axis

  • Add support in the generator

  • Update documentation

  • Annotate existing fields that already rely on custom accessors

Rationale

The previous annotation scheme could not clearly express:

  • Direction (read vs write)
  • Implementation (generated vs custom)

This becomes important for:

  • Python/SWIG binding generation
  • Consistency checks between C and bindings

These changes establish a clear and extensible model without altering existing runtime behavior.

Notes

  • No functional changes intended
  • Changes are kept incremental for easier review
  • This is groundwork for follow-up patches adding binding generation

Feedback welcome before building on top of this.

Rename the member-level annotation prefix from !accessors: to !access:
to decouple it from the !generate-accessors struct-level annotation.

Until now, the !accessors:<value> member annotation was consumed only
by the accessor generator, and its name reflected that — it read as a
natural companion to the struct-level !generate-accessors annotation.

That one-to-one relationship is about to change. An upcoming
!generate-python struct-level annotation will also need to read the
same member annotations, both to drive the generated SWIG layer and to
validate consistency between the C and Python sides. The annotation is
no longer the private concern of the accessor generator.

Keeping the name !accessors: would make that relationship confusing:

  - The visual similarity to !generate-accessors suggests the member
    annotation is owned by accessor generation, when in fact multiple
    generators will consume it.
  - Readers (and future contributors adding more generators) would
    reasonably — but wrongly — assume the annotation is accessor
    specific and only relevant when !generate-accessors is set.

Renaming to !access: breaks that false association. The new name also
reads more naturally as English:

  char *name;  // !access:readonly
  char *addr;  // !access:none

vs.

  char *name;  // !accessors:readonly
  char *addr;  // !accessors:none

This patch is a pure rename with no behavioural change:

  - generate-accessors.py: update has_annotation() lookups and the
    docstring examples
  - private.h: rename all 47 member annotations
  - generate-accessors.md: update every example, table row and note;
    re-pad the affected table rows to preserve column alignment

Generator output (accessors.h / accessors.c / accessors.ld) is byte
identical before and after this commit.

Signed-off-by: Martin Belanger <[email protected]>
Assisted-by: Claude Opus 4.7 <[email protected]>
Comment thread libnvme/src/nvme/private.h Outdated
Comment thread libnvme/src/nvme/private.h Outdated
@martin-belanger
Copy link
Copy Markdown
Author

If it helps, we could also consider making write=none the default in some structs where setters are rarely needed, which would allow shorter forms like !access:read=custom without ambiguity.

I can make that change if you'd like.

Replace the single-mode !access:<value> annotation with an orthogonal
two-axis spec:

  !access:read=<mode>,write=<mode>

where read and write are independent and each takes one of:
generated (generator emits the accessor), custom (hand-written
accessor in the public API, generator emits nothing), or none (no
accessor exists).  The same spec is accepted at the struct level via
!generate-accessors:read=<mode>,write=<mode>, with per-axis
inheritance from the struct-level default to member-level overrides.

The new model lets downstream consumers (the Python-binding generator,
the nvme.i consistency check) distinguish "no accessor" from
"hand-written accessor", and cleanly expresses mixed cases such as
"generated getter + hand-written setter" that the old one-dimensional
annotation could not name.  This is a clean break: the old :none /
:readonly / :writeonly / :readwrite tokens are no longer recognised.

See libnvme/tools/generator/generate-accessors.md for the full syntax,
inheritance rules, and examples.

All 47 !access: annotations in private.h are updated accordingly.  The
regenerated accessors.{h,c,ld} are byte-identical to the pre-change
baseline.

Signed-off-by: Martin Belanger <[email protected]>
Assisted-by: Claude Opus 4.7 <[email protected]>
@martin-belanger martin-belanger force-pushed the libnvme-python-bindings-redesign-plan-1 branch from 290e916 to 1a0ccef Compare April 24, 2026 14:34
@martin-belanger
Copy link
Copy Markdown
Author

martin-belanger commented Apr 24, 2026

By setting the struct-level defaults I was able to reduce the number of member-level overrides.

BTW, we may need to revisit the member annotations. I'm not sure that we need accessors for all of them For example, we are generating accessors for libnvme_path::curr_idx, but this looks like an internal parameter. I'm not sure that needs to be exposed through the public API at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants